fix: sessions flakes #15629
fix: sessions flakes #15629
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix flaky tests by adding sync blocks to wait for background queue operations. The approach is sound and should improve test stability. The added block in setUp() correctly isolates tests by draining the queue from previous runs. I've added one comment on the second added block in runSessionsSDK() to suggest improving a comment for clarity and to point out code duplication that could be refactored.
FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift
Outdated
Show resolved
Hide resolved
FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift
Outdated
Show resolved
Hide resolved
FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift
Outdated
Show resolved
Hide resolved
2 participants
GHA seems to flake when tests try to await (via XCTestExpectation) work that is done with the background priority. This queue has low priority scheduling that seems to very delayed in a test runner environment. This fix for this problem in the Crashlytics test suite (#15623) was to inject a higher priority queue during testing to ensure the awaited work gets done quickly. Since the queue in question is a concurrent queue, there is no way to reliably "drain" it as we could do with a serial queue. Should this fix start to flake in the future, we could inject a serial queue and drain it, but keeping it a concurrent queue, albeit with a higher priority seems IMO more similar to how it would run in non-test environment.
The flakes are most likely due to:
firebase-ios-sdk/FirebaseSessions/Sources/FirebaseSessions.swift
Line 182 in 54a9296
#no-changelog